Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

apps: add apps propose command, expand spec validation #926

Merged
merged 2 commits into from
Apr 13, 2021
Merged

apps: add apps propose command, expand spec validation #926

merged 2 commits into from
Apr 13, 2021

Conversation

kamaln7
Copy link
Contributor

@kamaln7 kamaln7 commented Dec 9, 2020 •
edited
Loading

  1. Added doctl apps propose
  2. Updated doctl apps spec validate
  3. Updated other commands to accept reading app specs from stdin by setting the path to -

doctl apps propose

API docs: https://developers.digitalocean.com/documentation/v2/#propose-an-app-spec

Usage: doctl apps propose --spec spec.yaml [--app app-id]

available app name

App Name Available?    Is Static?    Free Static Site Usage    $/month    $/month on higher tier    $/month on lower tier
yes                    no            3 of 3                    5.00       17.00                     n/a

unavailable app name

App Name Available?    Suggested App Name     Is Static?    Free Static Site Usage    $/month    $/month on higher tier    $/month on lower tier
no                     sample-dockerfile-2    no            3 of 3                    5.00       17.00                     n/a

doctl apps spec validate

Added a --schema-only flag. Defaults to false. If set, the old behavior of only validating that the spec marshals into a godo.AppSpec object is retained.

If not set, it calls the ProposeApp API endpoint.

In both cases the command now prints the validated spec instead of a The app spec is valid message.

some file that's not a spec

Error: Failed to parse app spec: json: cannot unmarshal string into Go value of type godo.AppSpec
exit status 1

valid yaml but invalid spec

Error: POST https://api.digitalocean.com/v2/apps/propose: 400 error validating app spec field "services.git.repo_clone_url": services.git.repo_clone_url in body is required
exit status 1

happy path - valid spec

The spec I passed in doesn't set the routes field

name: sample-hugo
region: nyc
static_sites:
- build_command: hugo
  environment_slug: hugo
  envs:
  - key: HUGO_VERSION
    scope: BUILD_TIME
    value: 0.74.3
  git:
    branch: main
    repo_clone_url: https://github.com/digitalocean/sample-hugo.git
  name: hugo
  routes:
  - path: /

Base automatically changed from master to main January 20, 2021 19:04
@kamaln7 kamaln7 force-pushed the apps-proposeapp branch 3 times, most recently from cd96d97 to f29cc31 Compare March 26, 2021 17:56
@kamaln7 kamaln7 marked this pull request as ready for review March 26, 2021 17:56
ChiefMateStarbuck
ChiefMateStarbuck previously approved these changes Apr 5, 2021
View reviewed changes
Copy link
Contributor

@ChiefMateStarbuck ChiefMateStarbuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! I'll need an approval from another app engineer to merge this

andrewsomething
andrewsomething reviewed Apr 13, 2021
View reviewed changes
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! One comment in line. Let me know how you want to approach it. If you're fixing on the API side, I'll go ahead and merge.

commands/displayers/apps.go Outdated
}

func (r AppProposeResponse) KV() []map[string]interface{} {
staticSites := fmt.Sprintf("%s of %s", r.Res.ExistingStaticApps, r.Res.MaxFreeStaticApps)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the API does not return the existing_static_apps field at all if no static sites are in use. Since it is a string, not an int, the default value is blank rather than 0. So you end up with:

Free Static Site Usage
of 3

You could handle it here, but you might want to fix that on the API side.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @andrewsomething! I updated the representation of free static apps

Kamal Nasser added 2 commits April 13, 2021 19:40
andrewsomething
andrewsomething approved these changes Apr 13, 2021
View reviewed changes
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@andrewsomething andrewsomething merged commit 833831a into digitalocean:main Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewsomething andrewsomething andrewsomething approved these changes

@ChiefMateStarbuck ChiefMateStarbuck ChiefMateStarbuck left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kamaln7 @andrewsomething @ChiefMateStarbuck